Conversation
…irect P0s) PR #337 merged 31 handler shims that closed the AudioStack 404 issue but Emma's sales-direct backend test (verdict 2/10) surfaced three P0s downstream of the shim layer: every MCP ``tools/call`` 500'd with ``'dict' object has no attribute 'account'``, and buyer-registered push_notification_config.urls were silently dropped when the adopter didn't wire ``webhook_sender``. Unit tests passed because they bypass ``create_tool_caller`` and call shim methods directly. handler.py — moved every ``adcp.types`` Request/Response import out of ``if TYPE_CHECKING:`` so ``typing.get_type_hints()`` resolves the forward refs at runtime. Without this, the dispatcher's typed-params resolver raises ``NameError``, falls back to dict dispatch, and the shim's ``params.account`` access crashes. webhook_emit.py — warn when ``sender=None`` but the buyer registered ``push_notification_config.url``. Adopters who skip ``webhook_sender`` in ``serve()`` now see a WARNING citing the buyer's URL on first call instead of having buyers complain that their notifications never arrive. mcp_tools.py — bumped ``_resolve_params_pydantic_model`` fallback log from DEBUG to WARNING with explicit remediation guidance (import the model at module scope vs. declare ``params: dict``). Silent dispatch fallback is what hid the wire-dispatch break for two PRs. tests/test_decisioning_wire_dispatch.py (new, 29 tests) — pins: - ``typing.get_type_hints`` resolves on every shim - ``_resolve_params_pydantic_model`` returns the typed Pydantic class - ``create_tool_caller`` round-trips a wire-shape dict through the full dispatch path without the params.account crash Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three reviewers (code-reviewer, python-expert, adtech-product-expert) converged on five P1/P2 fixes before merge. Wire-dispatch test: - Parametrize over ``PlatformHandler.advertised_tools`` so any new shim auto-extends regression coverage. Was 14 hardcoded tools (28 cases); now 40 (80 cases). - Pin the typed-dispatch contract end-to-end: assert the platform method actually receives a Pydantic ``GetProductsRequest`` / ``BuildCreativeRequest``, not a raw dict. - Drop hardcoded class-name assertions in favor of ``issubclass(resolved, BaseModel)``. Resolver warning (mcp_tools.py): - Surface the failing class name from ``NameError.name`` directly so adopters don't have to parse the bound-method repr. - Trim historical reference per CLAUDE.md. - Forward-compat note for PEP 749 (3.14 lazy annotations). F12 webhook silent-drop (webhook_emit.py): - Move the ``sender=None`` warning ABOVE the full URL/token extraction. A weird ``params`` shape that makes ``_extract_push_notification_url_and_token`` raise mid-traversal would otherwise re-introduce the silent-drop the previous fix was supposed to eliminate. Cheap pre-check on ``getattr(params, "push_notification_config", None)`` decides whether to warn; URL extraction is best-effort for log context. Test count: 2832 passed (was 2777 — +55 from wider parametrize coverage). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
added a commit
that referenced
this pull request
May 1, 2026
…es) (#339) * feat(decisioning): per-specialism tools/list filter (Emma cross-cutting P1) Three independent Emma backend tests (sales-direct, AudioStack/creative, signals-marketplace, Stability AI/creative) all flagged the same bug: ``tools/list`` advertises 40+ tools regardless of the platform's claimed specialisms. A sales-only adopter saw ``acquire_rights``, ``build_creative``, ``check_governance``; on every call buyers got NOT_SUPPORTED. The override-detection filter (``_is_method_overridden``) walks ``PlatformHandler.__mro__`` and finds the class concretely defines all 40+ shims — every tool shows as "implemented" regardless of what the underlying platform claims. Fix: add ``advertised_tools_for_instance(self) -> frozenset[str]`` on PlatformHandler. The framework's ``get_tools_for_handler`` checks for this hook on instances and intersects the candidate set with the per-instance result BEFORE the override-detection filter. The hook maps each claimed specialism to its per-Protocol-family advertised set via ``SPECIALISM_TO_ADVERTISED_TOOLS``. Empty per-instance set (novel specialism slug not in the map) falls back to the class-level universe — muting the handler entirely on a forward-compat slug would be worse than over-advertising. Static inspection by class also keeps the full universe so storyboard tests and spec-conformance docs aren't disrupted. Tests: 9 new (drift guards, per-specialism leak guards for sales/signals/creative/hybrid, novel-specialism fallback, advertise_all interaction, class-level inspection). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(decisioning): INTERNAL_ERROR breadcrumb on wire (Emma AudioStack P2) Adopters debugging "Error executing tool X: AdcpError[INTERNAL_ERROR / terminal]: An internal error occurred" had no wire-side breadcrumb — they had to grep server logs to even see which exception class fired. The Emma AudioStack backend test (verdict 6/10) explicitly flagged this: "the wire side An internal error occurred is a dead end." Add ``details.caused_by`` to the wire envelope when the framework wraps a non-AdcpError exception: { "code": "INTERNAL_ERROR", "message": "Platform method 'build_creative' raised AttributeError; see details for cause", "recovery": "terminal", "details": { "caused_by": { "type": "AttributeError", "message": "'dict' object has no attribute 'message'" } } } Exposes class name + truncated str (200 char cap) — no traceback, no module path, no chained __cause__. Full repr stays in server logs via ``logger.exception``. Truncation is defense-in-depth against an adopter who throws on secret material with a sloppy repr; the cap prevents secret-shaped values from landing on the wire. Applied to all three INTERNAL_ERROR wrap sites (sync method, non-projected TypeError, handoff fn). Drift guard: a unit test verifies the truncation cap matches the constant. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(decisioning): boot-time webhook_sender fail-fast (Emma F12 P1) Adopters claiming any specialism whose tool surface includes a spec-eligible webhook task type (``create_media_buy``, ``activate_signal``, ``acquire_rights``, etc.) but who skip ``webhook_sender`` would have every buyer-registered ``push_notification_config.url`` silently dropped. PR #338 added a runtime WARNING on first call; this commit adds the boot-time fail-fast that adtech-product-expert called for — "the same posture as ``validate_platform``'s governance opt-in gate." ``adcp.decisioning.serve.create_adcp_server_from_platform`` now calls ``validate_webhook_sender_for_platform`` after the handler is constructed. Uses the per-instance advertised set (NOT the class-level universe), so test fixtures with no claimed specialisms — and discovery-only agents — don't accidentally trip the gate. Adopter remediation paths surfaced in the error: * Wire a configured ``WebhookSender``. * Or set ``auto_emit_completion_webhooks=False`` if handling webhooks manually. Tests: 4 new gate behaviors. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(examples): per-Protocol-family hello_seller_*.py templates Every Emma backend test (sales 2/10, AudioStack 6/10, Signals 8/10, Stability 5/10) flagged "no example for my specialism" as P1 friction. Adopters writing creative/signals/audience/governance/etc. agents had only ``hello_seller.py`` (sales-non-guaranteed) plus the Protocol's 170-line docstring — adtech-product-expert called this the "highest-leverage follow-up after tools/list." Eight new example files, one per non-sales Protocol family: * ``hello_seller_creative.py`` — CreativeBuilderPlatform. Bare CreativeManifest projection, AudioStack/Stability shape. * ``hello_seller_signals.py`` — SignalsPlatform. Catalog + sync activate + TaskHandoff template. * ``hello_seller_audience.py`` — AudiencePlatform. Demonstrates arg-projection ergonomics. * ``hello_seller_governance.py`` — CampaignGovernancePlatform. governance_aware=True opt-in + 4 required methods. * ``hello_seller_brand_rights.py`` — BrandRightsPlatform. 4-arm acquire_rights discriminated union. * ``hello_seller_content_standards.py`` — ContentStandardsPlatform. 6 required + optional UNSUPPORTED_FEATURE gating. * ``hello_seller_property_lists.py`` — PropertyListsPlatform. In-memory CRUD + fetch-token + security-critical delete. * ``hello_seller_collection_lists.py`` — CollectionListsPlatform. Mirror of property-lists for collections. Each example fits in <100 lines, runs standalone, uses canonical type names (``CreativeManifest``, ``AudioContent``, ``FormatReferenceStructuredObject``). The ``AudioContent`` callout in the creative example documents the v4.0 payload/slot naming split that Emma's AudioStack adopter tripped on. Tests: 8 new — boot each example via PlatformHandler and verify ``advertised_tools_for_instance()`` narrows to the specialism's tools without leaking to other Protocol families. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(decisioning): expert-review fixes on PR #339 + DX polish Code-reviewer + adtech-product-expert second-pass on PR #339. Three P0/P1 findings + the deferred port-collision and /mcp redirect work folded into the comprehensive bundle. P0 fix (code-reviewer): - All 10 hello_seller_*.py examples called ``serve()`` without ``webhook_sender``. The new A3 boot-gate (this PR) rejects them because their advertised tools are in SPEC_WEBHOOK_TASK_TYPES. Run instructions in each example crashed at boot. Fix: pass ``auto_emit_completion_webhooks=False`` with a comment teaching adopters where to wire ``webhook_sender=`` in production. New smoke test ``test_example_boots_via_create_adcp_server_from_platform`` catches this regression class going forward. P1 fix (adtech-product-expert): - Boot-time webhook gate raised ``ValueError``; now raises ``AdcpError(INVALID_REQUEST)`` for parity with ``validate_platform``'s sibling boot-time gates (governance opt-in, missing required methods). Adopter ``except AdcpError`` clauses catch all platform-config failures uniformly. ``details.missing`` + ``details.webhook_eligible_tools`` for programmatic remediation. Deferred → folded in: - **Port-3001 EADDRINUSE friendly remediation** (2-of-4 Emma reports). ``_bind_reusable_socket`` projects EADDRINUSE OSError to a remediation-bearing message citing the busy port + ``port=`` / ``ADCP_PORT`` knobs. Other OSErrors (perm denied, address-not-avail) pass through unchanged so adopters debugging a different problem don't get a misleading port-collision message. - **/mcp vs /mcp/ 307 redirect** (2-of-4 Emma reports). New ASGI middleware ``_wrap_with_path_normalize`` strips trailing slashes before dispatch. Buyer libs POSTing to ``/mcp/`` now route to the same handler as ``/mcp`` without the 307 (which silently broke libs that don't follow redirects on POST — they revert to GET on the redirected URL, losing the body). Root path ``/`` left alone to avoid health-check 404. Scope-copy semantics preserved so outer middlewares aren't affected. Tests: 2868 pass (was 2854). 6 new (2 port-collision, 4 path-normalize) + 8 new example-boot smoke tests. F12 gate test updated to assert ``AdcpError("INVALID_REQUEST")`` + structured ``details``. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
bokelley
added a commit
that referenced
this pull request
May 1, 2026
…341) * fix(decisioning): three Emma cross-cutting findings (post-#340 matrix run) The post-#340 Emma matrix run (sales 8/10, signals 9.5/10, stability 8/10) surfaced three items every working backend flagged: A1 — base.py TYPE_CHECKING leak. ``ADCPHandler.get_adcp_capabilities`` (and every other framework baseline method) had its Request type imported under ``if TYPE_CHECKING:``. The dispatcher's ``_resolve_params_pydantic_model`` calls ``get_type_hints`` at boot; forward refs failed, ``NameError`` got swallowed, every adopter saw ``"typed params annotation failed to resolve … unresolved name: GetAdcpCapabilitiesRequest"`` on cold boot. The remediation text told adopters to import a model — but they CAN'T, it's not their method. Same bug class as PR #338's handler.py fix; we missed base.py because the framework methods inherit ``not_supported`` defaults so the wire-failure mode was invisible until #338's resolver-warning bump made it console noise. A2 — example field-name drift. Three examples referenced fields that don't exist on the actual schemas: * ``hello_seller_signals.py:105`` read ``req.signal_id``; ``ActivateSignalRequest`` carries ``signal_agent_segment_id``. Pre- fix: silent ``dep-unknown`` deployments. Now correctly reads the spec field AND iterates buyer-supplied ``destinations``. * ``hello_seller_creative.py`` docstring said ``req.brief`` / ``req.format_id``; ``BuildCreativeRequest`` actually carries ``message`` / ``target_format_id``. * ``hello_seller.py`` lacked a buyer-side request example. Adopters copy-pasted seller stub and got INVALID_REQUEST. Added a "Minimum valid buyer payloads" docstring section with a worked ``create_media_buy`` payload. A3 — narrowed ValidationError details on the wire. When a platform method raises ``pydantic.ValidationError`` directly (typically the seller's code constructed an invalid response), the wire used to say "see details for cause" with empty details. PR #340's narrowing was server-log only. ``_internal_error_details`` now special-cases ValidationError and populates ``details.validation_errors`` with the narrowed field-path list (filtered through ``narrow_union_errors``). Defensive: a narrowing bug never 500s the wire — fall back silently to ``caused_by`` only. Plus: ``scripts/run_emma_matrix.sh`` (the runner that surfaced these findings) ships in the repo for future re-validation. Tests: 1 new dispatch test pinning ValidationError surfacing. ``test_call_tool_invokes_handler`` updated to send a minimum-valid ``GetProductsRequest`` payload now that typed dispatch correctly rejects empty dicts. Test count: 2879 passed (was 2878). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(server): surface AdcpError details on MCP wire (AudioStack Emma P0) Decisioning AdcpError isn't a subclass of adcp.exceptions.ADCPError, so 'except ADCPError' in serve.py didn't catch it — propagated to FastMCP's default handler and details was silently dropped on the MCP wire (A2A buyers got it via data.details, MCP buyers got nothing). Three-part fix: 1. translate.py:translate_error — recognize decisioning AdcpError (lazy import) and extract code/message/suggestion/recovery/details/ field. ADCPTaskError branch also lifts errors[0].details so create_tool_caller's INVALID_REQUEST projection (with #341's narrowed validation_errors) reaches MCP buyers. 2. translate.py:_to_mcp — accept details and serialise as a '\nDetails: <json>' suffix in the ToolError text payload. Truncated to 8 KB. 3. serve.py:_call_handler — follow-on except branch catches decisioning AdcpError (lazy import + isinstance) and routes through translate_error. Tests: 3 new pinning caused_by + validation_errors surfacing and the 8 KB truncation. Test count: 2882 passed (was 2879). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Emma's sales-direct backend test (verdict 2/10) caught three P0s that PR #337's shim work didn't surface. Unit tests pass because they bypass
create_tool_callerand call shim methods directly; this PR adds wire-path coverage and fixes the underlying bugs.handler.py— types out ofTYPE_CHECKING:typing.get_type_hints()raisedNameErroron the forward refs, dispatcher fell back to dict path, every wiretools/callcrashed with'dict' object has no attribute 'account'. Fix: moveadcp.typesimports to module scope.webhook_emit.py— F12 silent no-op:sender=None+push_notification_config.urlset → silent drop. Now logs WARNING citing the buyer URL.mcp_tools.py— resolver log: bumped fallback from DEBUG → WARNING with explicit remediation. Silent dispatch fallback is what hid the wire-dispatch break for two PRs.Test plan
tests/test_decisioning_wire_dispatch.py(new, 29 tests) —get_type_hints+ resolver + end-to-endcreate_tool_callerdispatchtests/test_decisioning_webhook_emit.pyupdated — asserts WARNING when sender=None + URL set; silent skip when sender=None + no URLDeferred to follow-up PRs
_is_method_overriddenruns onPlatformHandlernot_platform, so a sales-only adopter sees all 40 tools advertised.examples/README clarifying high-level (hello_seller.py) vs low-level (seller_agent.py) archetypes.🤖 Generated with Claude Code